Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Tree provider uniqueness #930

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from
Draft

Conversation

kgilpin
Copy link
Contributor

@kgilpin kgilpin commented Mar 29, 2024

Problem

Note the inclusion of the same AppMaps in two different projects:

Screen Shot 2024-03-29 at 5 55 55 PM Screen Shot 2024-03-29 at 5 55 49 PM

Solution

AppMaps no longer replicated across projects:

Screen Shot 2024-03-29 at 1 31 37 PM

AppMap collections / sub-groups are
non-unique due to lack of a defined id, and therefore replicated
across all projects in a workspace.

This PR groups appmaps uniquely within tree items.
@kgilpin kgilpin marked this pull request as draft March 29, 2024 17:32
@kgilpin kgilpin self-assigned this Mar 29, 2024
@kgilpin kgilpin added the bug Something isn't working label Mar 29, 2024
const lightChangedIcon = join(__dirname, '../images/modified-file-icon-dark.svg');
const darkChangedIcon = join(__dirname, '../images/modified-file-icon-light.svg');
// const lightChangedIcon = join(__dirname, '../images/modified-file-icon-dark.svg');
// const darkChangedIcon = join(__dirname, '../images/modified-file-icon-light.svg');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is WIP, I will add this back in.

const listItems = this.appmaps
.appMaps()
.filter((appMap) =>
isDeepStrictEqual(AppMapTreeDataProvider.appMapFolderItems(appMap), folderProperties)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method doesn't distinguish AppMaps by what project they are in. It collects all AppMaps that have the same "folderProperties", but the project in which the AppMap is defined is not one of those properties. Therefore, AppMaps get duplicated across workspace projects in the tree view.

}
}

private getTopLevelTreeItems() {
const projects = vscode.workspace.workspaceFolders;
if (!projects || this.appmaps.appMaps().length === 0) return [];

const appmapsByProject = new Map<vscode.WorkspaceFolder, AppMapLoader[]>();
const projectsWithAppMaps = this.appmaps.allAppMaps().reduce((projectsWithAppMaps, appmap) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Should be appmaps.appMaps(), no?

@kgilpin
Copy link
Contributor Author

kgilpin commented May 16, 2024

I still haven't gotten around to fixing this. It affects VSCode users with multi-project workspaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant